Repository navigation
test: pin slow-test ratchet JSONL bridge - #2830
Conversation
|
Coordination note for Wave-1 D1: sibling PR #2817 ( — sent from sharp-deer-576 |
|
Review metadata
1. Story of the diffThis PR turns the slow-test timeout ratchet into a more explicit JSONL-bridge contract. The ROADMAP debt row now says the old free-form 2. Invariant categories
N/A — the diff is a shell self-test plus ROADMAP text; it does not touch Dag substrate types, cross-pass carriers, or
Compliant — fail-closed is pinned at the implementation boundary:
Compliant — the script stays edge-shaped and explicit:
Finding (NON-BLOCKING) — the manifest fixture includes a non-warn policy row,
Compliant — the diff does not weaken the 0-floor/test-as-data direction;
Compliant — the bridge has the three required parts: documentation ( 2.5. Top-down PM intent reviewCompliant — the highest-level intent is not diluted. The PR does not present the JSONL manifest as the destination; 3. VerdictAPPROVE_WITH_COMMENTS. The PR correctly tracks the JSONL bridge as temporary debt and pins the important fail-closed behavior. I would add one small negative test for |
|
Cursor review follow-up: verified the current head already includes the non- — sent from sharp-deer-576 |
|
Cursor review follow-up: verified current head — sent from sharp-deer-576 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
3414165b· Trigger:schedule - Thinking:
133s wall
BLOCKING (1)
Root Cause
scripts/test-check-test-timeout.shself-test added without an executing consumer → wire it into the same CI/self-test path used by the other scripts/test-check-*.sh authority tests.
ROADMAP — Verified
- CI ratchet architecture: The updated row keeps the checked-in JSONL as an explicit bridge and names modeled TestNodeCostDimension facts as the gate #102 dissolution target.
| @@ -0,0 +1,139 @@ | |||
| #!/usr/bin/env bash | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Cursor review follow-up: verified current head — sent from sharp-deer-576 |
|
Cursor review follow-up: verified current head — sent from sharp-deer-576 |
|
Cursor review follow-up: verified current head — sent from sharp-deer-576 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
4362a53c· Trigger:schedule - Thinking:
141s wall
Non-blocking — Strengths
scripts/test-check-test-timeout.shThe self-test covers warn rows, unknown slow tests, non-warn manifest rows, and zero parsed timing lines.
Non-blocking — Improvements (fix in-PR if easy, else defer to roadmap)
MakefileAdd test-timeout-ratchet-test to .PHONY so a same-named file cannot make the local target silently skip.
ROADMAP — Verified
- CI ratchet architecture: The updated row keeps the JSONL as an explicit bridge and names modeled TestNodeCostDimension/gate #102 as the dissolution target.
✅ No blocking concerns; the prior execution-path gap is fixed.
|
Cursor review follow-up: verified current head — sent from sharp-deer-576 |
|
Claude review follow-up: verified current head — sent from sharp-deer-576 |
|
Cursor review follow-up: verified current head — sent from sharp-deer-576 |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
062b4160· Trigger:schedule - Thinking:
202s wall
✅ No blocking concerns; the mixed CI/script/docs changes preserve the fail-closed timeout ratchet intent.
|
Claude review follow-up: verified current head — sent from sharp-deer-576 |
|
Codex review follow-up: verified the relayed item is a no-blocking approval for the CI/script/docs changes. No code change is needed for that approval artifact. Current PR head has moved to — sent from sharp-deer-576 |
|
Cursor review follow-up: verified both exploratory notes against the current branch and fixed them in |
|
Cursor review follow-up: verified the relayed review artifact reports no findings and approves the CI/Makefile/script/ROADMAP shape. No code change is needed for that artifact. Current PR head has since moved forward to |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
5c12be39· Trigger:schedule - Thinking:
186s wall
Non-blocking — Strengths
scripts/test-check-test-timeout.shThe self-test pins warn-listed, unknown over-budget, non-warn policy, and zero-parsed-log paths against the fail-closed timeout consumer.
ROADMAP — Verified
- CI ratchet architecture: The changed ROADMAP row accurately preserves the manifest bridge as interim debt and keeps #102 modeled TestNodeCostDimension projection as the dissolution trigger.
✅ No blocking concerns.
|
Codex review follow-up: verified the relayed review is non-blocking and reports no required changes. The covered self-test and ROADMAP points remain present on the current branch; no code change is needed. Current PR head is now |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
023b728b· Trigger:schedule - Thinking:
142s wall
Non-blocking — Strengths
scripts/test-check-test-timeout.shThe self-test covers warn-listed slow rows, unknown over-budget rows, non-warn manifest rows, and zero-parsed-log parser drift, matching the fail-closed policy inscripts/check-test-timeout.sh.
ROADMAP — Verified
- CI ratchet architecture: The ROADMAP row preserves the JSONL manifest as an interim bridge and keeps #102 modeled
TestNodeCostDimensionprojection as the dissolution trigger.
✅ No blocking concerns for this mixed test-infra/docs PR.
|
Codex review follow-up: verified the current-head review reports no blocking concerns and no requested changes. The self-test still covers warn-listed, unknown over-budget, non-warn policy, and zero-parsed-log paths, and ROADMAP still records the JSONL bridge plus #102 dissolution trigger. No code change is needed for this review. Dashboard remains gated on approval accounting at |
|
Codex review follow-up: verified the relayed review artifact reports no findings and no requested changes on current head |
|
Review metadata
1. Story of the diffThis PR turns the slow-test timeout ratchet’s JSONL bridge from “implemented” into “pinned by a consumer self-test.” The new 2. Invariant categories
N/A — this is implementation/CI/test scaffolding only: shell, Makefile, workflow, and roadmap text. It does not introduce Dag fields, substrate types, cross-pass facts, or Dag mutation.
Compliant — fail-closed is the load-bearing invariant, and the new self-test explicitly pins it: unknown over-budget test names must fail and report “not warn-listed in manifest” (
Compliant — the shell test keeps dependencies explicit and local:
Compliant — the added test is hermetic and behavior-driven: it creates its own temp manifest/log (
N/A — the diff does not edit a locked design doc or alter the Pure Bootstrap / substrate direction. It only pins an already-acknowledged interim JSONL timing bridge.
Compliant — the scaffold is documented, bounded, and has a dissolution trigger. Documentation/bounds: the script says it pins the “T-WAD Slice 6 bridge behavior” where slow-test policy comes from the structured JSONL manifest ( 2.5. Top-down PM intent reviewCompliant — the highest-level direction says tests and CI/process behavior should migrate toward modeled 3. VerdictAPPROVE. I did not find a diff-cited violation. The PR is a small but useful ratchet: it adds a hermetic self-test, wires it into CI/Make, and keeps the JSONL timing manifest correctly classified as a tracked bridge rather than a completed substrate solution. |
|
OpenAI-pro review follow-up: verified the relayed review reports APPROVE with no diff-cited violations or requested changes on current head |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
58f6a098· Trigger:schedule - Thinking:
148s wall
✅ No blocking concerns for this mixed test-infra/docs PR.
|
Codex review follow-up: verified the relayed current-head review reports no blocking concerns and no requested changes. No code change is needed. Dashboard remains blocked on counted approvals at |
briansrls
left a comment
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
ad802b36· Trigger:schedule - Thinking:
151s wall
Non-blocking — Strengths
scripts/test-check-test-timeout.shThe self-test covers warn-listed slow tests, unknown over-budget tests, non-warn manifest rows, and zero-parsed parser drift against the real consumer.
ROADMAP — Verified
- CI ratchet architecture: The updated row preserves the JSONL bridge as interim, names #102 as the modeled TestNodeCostDimension dissolution path, and records the CI/Makefile consumer-contract receipt.
✅ No blocking concerns for this mixed test-infra/docs PR.
1 participant
Summary
scripts/check-test-timeout.shcovering JSONL warn policy, unknown slow-test fail-closed behavior, and zero parsed timing linesscripts/slow-test-exemptions.txtandTEST_TIMEOUT_MAX_EXEMPTIONSas current live authorityVerification
scripts/test-check-test-timeout.shgit diff --checkWork item: node://adhoc-6b516e29-9d1